Skip to content

Conversation

JordanYates
Copy link
Contributor

@JordanYates JordanYates commented Aug 3, 2025

Add an option that excludes generation of a given path of .gcda files when running coverage testing. This can be useful when running testing coverage on downstream repos, which would otherwise result in large .gcda files for paths we don't care about filling up the disk.

The issue triggering this PR was huge portions of coverage dumps for my downstream repo being just MbedTLS data, which was ignored at the next processing step due to --coverage-basedir. The dumped data ended up being so large that something between QEMU and handler.log was having trouble actually saving it all, leading to test failures due to "GCOV_COVERAGE_DUMP_END" not being found in the file.

nashif
nashif previously approved these changes Aug 4, 2025
Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks reasonable.
btw, been looking into using semihosting instead of console dumps, see #94079

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds configuration options to limit coverage dump generation by allowing users to filter which files are included in .gcda files during coverage testing. This addresses issues where coverage dumps become excessively large due to unwanted library data (like MbedTLS) causing disk space issues and test failures.

  • Added a new configuration option COVERAGE_DUMP_PATH_EXCLUSIVE for path-based filtering
  • Restructured Kconfig to group coverage-related options under a conditional block
  • Removed redundant depends on COVERAGE_GCOV clauses

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@JordanYates JordanYates force-pushed the 250803_coverage_limit branch 2 times, most recently from bb1b246 to b631ef7 Compare September 17, 2025 11:37
@JordanYates
Copy link
Contributor Author

Updated the implementation to instead specify a single exclude pattern. It ended up being much harder than I originally realised to construct a pattern that captured only the files I wanted.
Since the only real problem was the MbedTLS files, a single exclude path does the job fine.
And also resolves the naming concerns.

kartben
kartben previously approved these changes Sep 26, 2025
@nashif
Copy link
Member

nashif commented Sep 26, 2025

need rebase

Add an option that excludes generation of a given path of `.gcda` files
when running coverage testing. This can be useful when running testing
coverage on downstream repos, which would otherwise result in large
`.gcda` files for paths we don't care about filling up the disk.

One example of using this is to exclude coverage outputs from
mbedtls files with the following:
`CONFIG_COVERAGE_DUMP_PATH_EXCLUDE="*modules/crypto/mbedtls*"`

Signed-off-by: Jordan Yates <[email protected]>
Copy link

@nashif nashif requested a review from kartben October 4, 2025 11:15
@nashif nashif merged commit d185f8e into zephyrproject-rtos:main Oct 5, 2025
26 checks passed
@JordanYates JordanYates deleted the 250803_coverage_limit branch October 5, 2025 10:21
@tristan-google
Copy link
Contributor

This change appears to be causing issues in our code coverage builds since fnmatch is not supported by all the libc options. Picolibc and most host-provided libraries have it, minimal libc does not, and I don't think newlib does either (less sure about that one, but I didn't see it in its docs)

I think we need to add some depends on clauses to this Kconfig option and guard the references to fnmatch and fnmatch.h in coverage.c

@tristan-google
Copy link
Contributor

Please take a look at #97148 as possible solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants